Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add urdf::Rotation unit test #51

Merged
merged 1 commit into from
Oct 4, 2014
Merged

Conversation

sbarthelemy
Copy link

demonstrating numerical issues with the quaternion/rpy conversion (#49)

demonstrating numerical issues with the quaternion/rpy conversion (ros#49)
@isucan
Copy link
Contributor

isucan commented Oct 1, 2014

Thanks for this contribution! @hsu @dirk-thomas -- do we want to also allow boos unit test framework, or do we want gtest?

@dirk-thomas
Copy link
Member

This patch seems to not register the executable as a test. So from the build system view it is just an executable which will never get invoked when running make run_tests.

In order to use anything other then gtest it should provide the same functionality which e.g. includes generation of xunit compatible result files etc. Therefore I would recommend to stick to gtest since catkin provides all of this out-of-the-box.

@sbarthelemy
Copy link
Author

Actually I first wrote the test using gtest.
However, this project does not uses catkin but raw CMake, and it's not clear to me how I should link with gtest (I found no package providing the libgtest .so on my ubuntu), so I backed up to boost.

Indeed CTest stuff or equivalent is missing to declare this binary as a test and get it launched by buildfarms.

Note that I'm primarily interested in showing issue #49 up (and getting it fixed), not in setting a test framework up.

Cheers

@dirk-thomas
Copy link
Member

I didn't recall that urdfdom is now a plain CMake project. Then catkin obviously does not matter and you are completely on your own with testing (and making it work / being executed on the farm).

@sbarthelemy
Copy link
Author

@isucan So, what would be the plan for integrating the tests?

isucan added a commit that referenced this pull request Oct 4, 2014
@isucan isucan merged commit 29cca7a into ros:master Oct 4, 2014
@traversaro
Copy link
Contributor

It is strictly necessary to add a dependency on a test framework? Given that this is quite a simple test, I guess we could just use a main that output EXIT_SUCCESS on test success or EXIT_FAILURE on test failure, and that the corresponding binary to generate a CTest test.
Alternatively we can add a CMake option off by default for building tests, so that the test framework is a dependency only if the user wants to build the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants